Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add result.all TypeScript definition #345

Merged
merged 2 commits into from
Jul 4, 2019

Conversation

ehmicky
Copy link
Collaborator

@ehmicky ehmicky commented Jul 2, 2019

Fixes #342

While the all property is defined on the resolved value or rejected error (where it's a string or buffer), we forgot it on the return value (where it's a stream).

@tomsotte

@tomsotte
Copy link
Contributor

tomsotte commented Jul 3, 2019

As per merge-stream current implementation the mixed all stream that we create it's a PassThrough.
For execa purpose I believe it's correct to limit its type to a Readable stream.

@ehmicky
Copy link
Collaborator Author

ehmicky commented Jul 3, 2019

Yes I think that it being a PassThrough is an implementation detail. From the user's perspective, it should be manipulated like stdout and stderr which are typed as Readable streams.

readme.md Show resolved Hide resolved
@ehmicky ehmicky force-pushed the bug/add-all-type-definition branch from 2bae969 to 0ce5ca9 Compare July 4, 2019 15:25
@ehmicky ehmicky force-pushed the bug/add-all-type-definition branch from 0ce5ca9 to 27b4c25 Compare July 4, 2019 16:09
@ehmicky
Copy link
Collaborator Author

ehmicky commented Jul 4, 2019

Fixed.

@sindresorhus sindresorhus merged commit aa070b8 into master Jul 4, 2019
@sindresorhus sindresorhus deleted the bug/add-all-type-definition branch July 4, 2019 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

all is missing on ExecaChildProcess
3 participants